docs: specify symmetric deposit bounceback mechanism#360
docs: specify symmetric deposit bounceback mechanism#360
Conversation
Recreates #329 which was closed due to force push on main. Rebased on current main branch. Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5473-3933-756c-8b65-60dfe41cb262
…call sites Updates the Rust ABI binding and all callers to include the new bouncebackRecipient parameter added by the bounceback spec. Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5473-3933-756c-8b65-60dfe41cb262
- Format .deposit() calls to multi-line across 5 files (cargo fmt) - Pass ZONE_OUTBOX_ADDRESS as 4th arg to ZoneInbox constructor in generate_zone_genesis.rs (was missing after Solidity constructor update) - Regenerate zone-test-genesis.json with updated ZoneInbox bytecode Co-authored-by: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
Reverts unrelated changes that were bundled with the deposit bounceback PR: - Restore zone_id=0 unscoped token support in auth validation - Restore 30-day (2592000s) max validity window (was changed to 30min) - Restore enforce_transfer rejecting on policy resolution failure - Restore enforce_mint doc comment about L1 enforcement deferral - Restore MockPolicyProvider::failing() test helper - Restore policy cache seeding in start_local_zone_with_fixture Co-authored-by: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
The zonePortal field in the auth token wire format is unrelated to the deposit bounceback feature. Reverts token format back to the original 29-byte layout (version + zoneId + chainId + issuedAt + expiresAt). Co-authored-by: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
…zonePortal) Reverts rpc.md to main — the zonePortal field, removal of unscoped tokens section, and 30-minute validity window were not part of the deposit bounceback feature. Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
Co-Authored-By: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
- Remove .amp/tools (benchmarking toolbox wrappers) - Remove .mergify.yml (CI config) - Remove tempo.nu (benchmarking script) - Revert Justfile portal-in-token auth changes - Revert rpc/Cargo.toml tempo-compat feature (not needed) - Restore ztip20.rs policy resolution error tests Co-authored-by: dankrad <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-019d5f52-e187-7554-b95f-59c7b789eb28
mattsse
left a comment
There was a problem hiding this comment.
now require an L1 address (bouncebackRecipient) that receives refunded tokens if zone-side processing fails
there's a potential issue when the bouncebackRecipient is no longer able to receive the tokens when the bounceback is attempted to be processed on L1 (transfer fails due to 403)
Resolve conflicts: - docs/pages/protocol/privacy/overview.md: accept deletion (docs site removed on main in #366) - IZone.sol and Rust crates auto-merged; bounceback spec additions coexist with main's updates. Made-with: Cursor
Resolve conflicts introduced by PR #355 (deposit counter) landing on main: Code conflicts: - crates/primitives/src/abi.rs: DepositMade keeps both bouncebackRecipient (bounceback branch) and depositNumber (main). - docs/specs/src/zone/IZone.sol: same — event signature carries both fields. - docs/specs/src/zone/ZonePortal.sol: * DepositMade emit site now passes bouncebackRecipient and thisDeposit. * WithdrawalBounceBack emit site uses the renamed event (from BounceBack) and includes thisDeposit as the new trailing depositNumber field. - docs/specs/test/zone/ZonePortal.t.sol: DepositMade expectation now includes both bouncebackRecipient (address(0)) and depositNumber (1). Regenerated crates/tempo-zone/tests/assets/zone-test-genesis.json via `tempo-xtask generate-zone-genesis` since both branches updated ZoneInbox bytecode (bounceback adds bouncebackRecipient handling; deposit counter adds lastProcessedDepositNumber). Spec: - docs/specs/zone_spec.md condensed IZonePortal stub: update DepositMade, EncryptedDepositMade, BatchSubmitted, WithdrawalBounceBack events to include depositNumber; add DepositBounceBack event; update deposit() and depositEncrypted() signatures to include bouncebackRecipient; add depositCount() and lastProcessedDepositNumber() view functions. Made-with: Cursor
The integration tests' start_from_l1() patches the ZoneInbox/ZoneConfig bytecode at test setup time by replacing Address::ZERO with the real L1 portal address (see crates/tempo-zone/tests/it/utils.rs — expects 4 occurrences in ZoneInbox and 1 in ZoneConfig). The previous regeneration baked 0xbb..bb as the placeholder, causing `expected 4 tempoPortal immutable(s) ... found 0` assertion failures in all start_from_l1 integration tests (demo_cross_zone, demo_shield_and_send, etc.). Regenerated with --tempo-portal 0x0000...0000 to match main's convention. Made-with: Cursor
Previous regeneration used stale solidity artifacts that produced a ZoneInbox missing the advanceTempo selector (d01e8d31). This caused the system tx to revert at 188 gas (no matching dispatcher branch). The new genesis deploys ZoneInbox at 8091 bytes with all 7 expected function selectors including d01e8d31 advanceTempo. Made-with: Cursor
There was a problem hiding this comment.
Can we split this into two PRs? The current PR updates the Solidity implementation but doesn't add any detail to the spec describing the deposit bounceback mechanism.
Can we split into two PRs where:
-
spec first PR that formally describes the mechanism (when bouncebacks trigger, the no-recursive-bounce rule, etc.)
-
We can handle the implementation PR with the reference contract and Rust changes.
Resolve conflicts with main (which renamed docs/specs -> specs/ref-impls,
bumped tempo dep, and migrated IERC20 -> ITIP20 in SwapAndDepositRouter):
- Cargo.toml / Cargo.lock: take main's newer tempo rev (bb08bb90).
- crates/tempo-zone/Cargo.toml: drop `tempo-compat` feature on tempo-alloy;
the feature no longer exists in the bumped tempo rev.
- specs/ref-impls/src/zone/SwapAndDepositRouter.sol: combine both sides
- use main's ITIP20 interface (renamed from IERC20)
- keep bounceback branch's bouncebackRecipient=msg.sender arg on
IZonePortal.deposit() and depositEncrypted() calls
- crates/tempo-zone/tests/assets/zone-test-genesis.json: regenerate with
current Solidity artifacts so ZoneInbox and ZoneOutbox bytecode pick up
both the bounceback changes (bouncebackRecipient, DepositBounceBack
event, renamed WithdrawalBounceBack) and main's deposit counter changes
(lastProcessedDepositNumber storage).
Made-with: Cursor
|
Superseded — split into two PRs per review feedback:
Closing this one in favor of the split. The implementation PR is stacked on top of the spec PR. |
Summary
Recreates PR #329 which was closed due to a force push on main. Rebased the diff on the current main branch.
Specifies a symmetric bounce-back mechanism for deposits, closing the gap where failed deposits had no recovery path. Addresses #167.
Key design decisions:
bouncebackRecipienton deposits: Bothdeposit()anddepositEncrypted()now require an L1 address (bouncebackRecipient) that receives refunded tokens if zone-side processing fails.ZoneInbox.advanceTempo()fails to mint, the zone creates a zero-fee, zero-callbackPendingWithdrawaltargetingbouncebackRecipient.bouncebackRecipient = address(0). Bounce-back withdrawals havefallbackRecipient = address(0).Closes #167
Co-Authored-By: dankrad [email protected]
Prompted by: dankrad